-
Notifications
You must be signed in to change notification settings - Fork 3
Experiment: Add support for fetch-post to wp_rs_cli #834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Vibe-coded with cursor-agent
To make things more generic for potential future subcommands that might need similar auth instead of making this too specific to FetchPost
So it could be made more generic and reusable for any future subcommands that might need such conversion too
48d9527
to
d21276a
Compare
As this is only used within the implementation details of that method so makes more sense rather than polluting the module namespace
- Renamed TargetSiteResolver to SiteApiType - Extract the logic to detect the type based on AuthArgs and url into a dedicated `detect_from_args` method - Extract the logic to build the right `ApiUrlResolver` and `WpAuthenticationProvider` for each `SiteApiType` into dedicated methods This helps simplfy a lot the code for `build_api_client` and makes everything more readable
Move the `FetchPostArgs` struct to the top near the other subcommand definitions for consistency with having everything related to subcommand arguments grouped in the same place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am having a bit of a bandwidth issue to find the time to review this PR line by line, but luckily the wp_rs_cli
is currently just an extension and isn't expected to be as polished as the main library. I am not trying to say that these changes are not polished, but rather, it'd be OK if there are minor improvement areas here & there.
I've skimmed through the changes and although I see some potentially fragile bits, such as resolve_post_id
function, I think this is a good addition overall and I'd be comfortable with us iterating on it as we use it.
Thanks for the PR @AliSoftware!
What
This adds a
fetch-post
subcommand towp_rs_cli
Why
First, this is obviously a useful feature to have for the CLI
But in particular, as discussed in recent conversations with Oguz in Slack (internal ref: p1754602450419219/1754596820.179099-slack-C08CUT5NT88), this would be especially useful to integrate the CLI into some of our current AI workflows—where we might want to ask our LLM like Claude or Cursor to use the CLI to fetch a post's content and comments and do something with the data (summarize, extract particular info from comments and reformat, etc…)
How
Note
This is mostly vibe-coded with cursor-agent and GPT-5 🤖
(though I reviewed the code and made some tweaks of my own too)
AuthArgs
struct for common parameters related to authentication needed for this subcommand but that might also be useful to any future subcommand that might require authenticationbuild_api_client
method to build aWpApiClient
from theAuthArgs
and an optional pageurl
.SiteApiType::detect_from_args
will use thewpcom_site
(WpCom) orapi_root
(WpOrg) args fromAuthArgs
if explicitly providedurl
parameter ifwpcom_site
/api_root
are not provided explicitly (extractwpcom_site
from the host of theurl
if it's a*.wordpress.com
URL, useWpLoginClient.api_discovery(url)
to setapi_root
otherwise)Commands::FetchPost
subcommand, with its dedicatedFetchPostArgs
, a subset of them coming from#[command(flatten)] auth: AuthArgs
resolve_post_id
to find thepost_id
from a post URL / slug.fetch_post_and_comments
(the core implementation of theCommands::FetchPosts
command) to call theWpApiClient
's.posts().retrieve_with_view_context()
+.comments().list_with_view_context(…)
+ paginationwhile
loop in thefetch_post_and_comments
method. I didn't want to overcomplicate and try to optimize too early though, and figured the current approach would be ok as a first iteration of the implementation before it could be made more generic laterWpApiClient
's level that I missed?Testing
Future directions
Since this is the first of probably many future CLI subcommands that might require authentication, it'd be nice next to have a subcommand to perform the OAuth flow.
Then once we have automatic handling of OAuth flow (call
/authorize
, obtain the code, exchange it for a token, store the bearer token + refresh token + expiration locally for next CLI calls to reuse, auto-refresh the token on expiration…) by the CLI itself, theAuthArgs
and friends will likely be adjusted accordingly so that you don't have to pass the bearer token manually.But all that was out of scope for this spike/experiment, so for now you're just expected to obtain the bearer token on your end and provide it to the CLI, and we can refactor that part around
AuthArgs
later when we get to it or add more subcommands requiring it.